fix(sendingpolicy): lock the agent FOR NO KEY UPDATE in the accept path - #1006
Merged
Conversation
The v1.9.0 staging conformance gate failed on eight parallel HITL holds: SQLSTATE 40P01. Each accept transaction inserts its message first, which takes a FOR KEY SHARE lock on the agent row through the foreign key and a row lock on account_usage through the storage trigger, then prepares its operation, which locked the agent FOR UPDATE. FOR UPDATE conflicts with KEY SHARE, so two concurrent sends waited on each other. The direct send path (PrepareExternalTx) has the identical shape and deadlocks the same way under parallel sends; staging simply never ran that case. FOR NO KEY UPDATE keeps every ordering the gate needs (callers serialize against each other and against any update or delete of the row) and does not conflict with a foreign-key share. Same change for the webhook row. Two regression tests reproduce the deadlock deterministically at the gate and through the API, and both fail with the old lock. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX
…g it Review of the lock-order fix: the gate test's 300ms sleep could let A finish before B ever blocked, passing vacuously against the bug. B now reports its backend pid and A waits until pg_stat_activity shows it blocked on a lock. The e2e test no longer calls t.Fatal from worker goroutines, and the PrepareExternalTx ordering comment now describes the function rather than every caller. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX
The staging gate only sent in parallel from a HITL agent; the direct accept path has the same insert-then-lock shape and carries almost all traffic. Add the eight-parallel-direct-sends case, write the accept transaction's lock order into the pipeline design doc, and record the FOR KEY SHARE / FOR UPDATE rule in AGENTS.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The v1.9.0 staging release pipeline failed its conformance gate on
03-concurrency: 8 parallel sends from HITL agentwith HTTP 500failed to hold message for approval. Staging Postgres logs showdeadlock detected (SQLSTATE 40P01):Cause. Each accept transaction inserts the message first. The insert takes
FOR KEY SHAREon the agent row (foreign key) and a row lock onaccount_usage(storage trigger). It then prepares the sending operation, which locked the agentFOR UPDATE.FOR UPDATEconflicts withKEY SHARE, so with two concurrent sends A waits for B's key share while B waits for A'saccount_usagerow.The direct-send path (
PrepareExternalTx, B6) has the identical shape. Staging has no parallel direct-send test, so it passed by omission; the new API-level test reproduces the deadlock there too.Fix.
FOR NO KEY UPDATEon the agent (and webhook) row inPrepareExternalTx/PrepareNotificationTx. It still serializes gate callers against each other and against any update or delete of the row, which is all the ordering the design needs, and it does not conflict with a foreign-key share.Tests.
TestPrepareDoesNotDeadlockAgainstConcurrentInsert(sendingpolicy, deterministic two-transaction interleaving, both prepare paths) andTestParallelSendsFromOneAgentAllAccept(e2e, eight parallel API sends). Both fail withFOR UPDATErestored and pass with the fix.Blocks the v1.9.0 promotion; the release will be re-cut as v1.9.1 on this fix.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AjfGxvXW6fNKWGFHuo68yX